Rename CreateConditionCmd class to CreateConditionForVmAutoScalingCmd #11819
Conversation
boring-cyborg
bot
added
component:api
component:management-server
component:networking
labels
|
@erikbocks , sensible change but I would leave out the “Vm” bit from the new name. “CreateConditionForAutoScalingCmd” seems more appropriate as the VM is not scaled itself but the service is scaled by adding more (or less) VMs.What do you think? cc @weizhouapache @shwstppr . |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #11819 +/- ##
=========================================
Coverage 17.56% 17.56%
+ Complexity 15542 15541 -1
=========================================
Files 5909 5909
Lines 529058 529058
Branches 64617 64617
=========================================
+ Hits 92922 92942 +20
+ Misses 425683 425661 -22
- Partials 10453 10455 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
maybe use createAutoScalePolicy |
|
@DaanHoogland I don't have a strong preference either way. I don't even mind class remain CreateCondition as personally I find it easier during troubleshooting to search the class with API name. |
|
I like @weizhouapache ’s name best, |
|
I agree with the |
can you please also change the other classes for AS condition ? Update, Delete, List ? |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
erik-bock-silva
force-pushed
the
rename-create-condition-command-class-name
branch
from
16a6b0f to
bdce6c0
Compare
|
@DaanHoogland @weizhouapache, I renamed the other classes names, could you check them now? |
...java/org/apache/cloudstack/api/command/user/autoscale/ListAutoScaleConditionCountersCmd.java
Outdated
Show resolved
Hide resolved
...va/org/apache/cloudstack/api/command/admin/autoscale/DeleteAutoScaleConditionCounterCmd.java
Outdated
Show resolved
Hide resolved
...org/apache/cloudstack/api/command/admin/autoscale/CreateCounterForAutoScaleConditionCmd.java
Outdated
Show resolved
Hide resolved
|
@shwstppr @weizhouapache , can we continue with this or are there any impediments? |
we can continue |
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15577 |
|
@blueorangutan test keepEnv |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-14738) |
5 participants
Description
Currently, the
createConditioncommand belongs to a class named asCreateConditionCmd. This class has a very abstract name and does not define what is the purpose of the condition. Thus, this PR renamesCreateConditionCmdclass toCreateConditionForVmAutoScalingCmd, helping developers to understand the class purpose without having to access it.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
I built the packages with the changes, applied them in my local environment and called the API
createConditionvia CloudMonkey and validated that everything works as intended. Unitary tests were also executed and they passed successfully.How did you try to break this feature and the system with this change?